Skip to content

Conversation

gamazeps
Copy link
Contributor

@gamazeps gamazeps commented May 7, 2017

Part of #29378

  • Adds information about the stack_size when using Builder. This might be considered too low level, but I assume that if someone wants to create their own builder instead of using thread::spawn they may be interested in that info.
  • Updates the thread::Thread structure doc, mostly by explaining how to get one, the previous example was removed because it was not related to thread::Thread, but rather to thread::Builder::name.
    Not much is present there, mostly because this API is not often used (the only method that seems useful is unpark, which is documented in [DOC] Improve the thread::park and thread::unpark documentation #41809).

Felix Raimundo added 2 commits May 7, 2017 19:31
- Copied the module documentation to `Thread`.
- Removed the example because it did not use any method of Thread.
@rust-highfive
Copy link
Contributor

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@gamazeps
Copy link
Contributor Author

gamazeps commented May 7, 2017

r? @steveklabnik

@carols10cents carols10cents added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 8, 2017
Copy link
Member

@steveklabnik steveklabnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some small details! thank you!

@@ -244,6 +244,11 @@ impl Builder {
/// Generates the base configuration for spawning a thread, from which
/// configuration methods can be chained.
///
/// If the [`stack_size`][stack_size] field is not specified, the stack size
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changing this to just have

[`stack_size`]

would follow convention

@@ -259,6 +264,8 @@ impl Builder {
///
/// handler.join().unwrap();
/// ```
///
/// [stack_size]: ../../std/thread/struct.Builder.html#method.stack_size
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that means writing

[`stack_size`]: ../../std/

here

@@ -714,33 +721,21 @@ struct Inner {

#[derive(Clone)]
#[stable(feature = "rust1", since = "1.0.0")]
/// A handle to a thread.
/// Threads are represented via the `Thread` type, which you can get in one of
/// two ways:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This first line needs to be a summary; this doesn't quite work. I think leaving the first line, adding a /// in between, and keeping this new stuff is 👍

@gamazeps
Copy link
Contributor Author

gamazeps commented May 9, 2017

Done :)

@steveklabnik
Copy link
Member

@bors: r+ rollup

thanks a ton!

@bors
Copy link
Collaborator

bors commented May 9, 2017

📌 Commit 323a774 has been approved by steveklabnik

@bors
Copy link
Collaborator

bors commented May 9, 2017

⌛ Testing commit 323a774 with merge 644fc40...

bors added a commit that referenced this pull request May 9, 2017
[Doc] improve `thread::Thread` and `thread::Builder` documentations

Part of #29378

- Adds information about the stack_size when using `Builder`. This might be considered too low level, but I assume that if someone wants to create their own builder instead of using `thread::spawn` they may be interested in that info.
- Updates the `thread::Thread` structure doc, mostly by explaining how to get one, the previous example was removed because it was not related to `thread::Thread`, but rather to `thread::Builder::name`.
  Not much is present there, mostly because this API is not often used (the only method that seems useful is `unpark`, which is documented in #41809).
@bors
Copy link
Collaborator

bors commented May 9, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: steveklabnik
Pushing 644fc40 to master...

@bors bors merged commit 323a774 into rust-lang:master May 9, 2017
@gamazeps gamazeps deleted the thread-struct-doc branch May 13, 2017 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants